Remove single view assumptions from window.dart
#38453
Conversation
a-wallen
force-pushed
the
refactor_fl_view
branch
2 times, most recently
from
c8592e2 to
e54d32d
Compare
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
I know, this is still a draft, just some fly-by comments.
| set onPlatformConfigurationChanged(VoidCallback? callback); | ||
|
|
||
| Iterable<FlutterView> get views; | ||
| Map<Object, FlutterView> get views; |
There was a problem hiding this comment.
We can't expose this as a Map. Maps are mutable and we don't want the framework/apps to be able to mutate this object. It's also an (unnecessary) API break.
We have however talked about exposing a new method, something like getViewById that allows you to, well, get a view by passing in an ID without exposing the entire Maps interface.
a-wallen
force-pushed
the
refactor_fl_view
branch
from
48cb970 to
7ca1816
Compare
flutter-dashboard
bot
added
the
platform-web
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
a-wallen
force-pushed
the
refactor_fl_view
branch
from
d63479c to
d8425c2
Compare
a-wallen
force-pushed
the
refactor_fl_view
branch
from
10076f9 to
4c659b1
Compare
lib/ui/platform_dispatcher.dart
Outdated
| const ViewConfiguration({ | ||
| this.view, | ||
| @Deprecated(''' | ||
| Renaming window to view since `FlutterWindow` has been removed from dart::ui. |
There was a problem hiding this comment.
- I don't understand this deprecation message. It should be something actionable in the form of "Use xxxx instead." For example, "Use the
viewargument instead. Thewindowargument has been renamed toview." - If it's a rename, shouldn't we point them to the same value? The two parameter should be exclusive (assert that only one of them is non-null) and be assigned to the same field (
_view) and both getter should return this field.
There was a problem hiding this comment.
Yeah, you're right 👍
There was a problem hiding this comment.
How about
`FlutterWindow` has been removed from dart::ui, and the window property is no longer semantically sound
use the `view` property instead.
There was a problem hiding this comment.
In my opinion the explanation is unnecessary here. Just tell the user how to change it. If you really want to explain it, add a link to the design doc, or to this PR with an expanded paragraph of description.
There was a problem hiding this comment.
Reopening as I don't think the second part was addressed:
If it's a rename, shouldn't we point them to the same value? The two parameter should be exclusive (assert that only one of them is non-null) and be assigned to the same field (_view) and both getter should return this field.
|
🎉 just saw the notification that this landed. Kudos! |
10 participants
Removes
FlutterViewto support multi-window.Fixes flutter/flutter#112204
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.